fix incorrect reported txid in txpool rpc #674
Conversation
4meta5
added
A0-pleasereview
There was a problem hiding this comment.
Indeed, thank you for tracking this down and fixing it.
This is also missing in our tests and that's why we didn't catch it, please check here, if you don't mind please merge in that branch here or add it manually.
tgmichel
added
A8-mergeoncegreen
Merged that branch. Found a bug in the unit test and fixed that as well. The tests now pass as expected. |
|
The manually launched CI was successful: https://github.com/PureStake/moonbeam/actions/runs/1116753208 But github doesn't seem to understand, so only someone who has the rights to master can manually merge this PR :/ |
|
@crystalin is off, but he can merge this next week when he gets back or before then if he sees this |
What does it do?
Fixes a bug where the Rpc Tx Pool reports incorrect transaction ids. This PR attempts to fix this issue.
What important points reviewers should know?
Blockscout explorer's pending transactions never align with the actual completed transactions. These incorrect transaction ids end up stuck as phantom pending transactions, until blockscout deems them as failed. This can be observed here: https://moonbase-blockscout.testnet.moonbeam.network/pending-transactions
The bug lies in the conversion from an ethereum::Transaction to ethereum::TransactionMessage and using the TransactionMessage's helper function to get the Tx hash. This helper derives the chain_id from the V field of the signature's RSV. It follows eip155 V values so the chain_id calculation formula causes a mismatch, which in turn causes the final rlp encoding/Tx hash to differ from the actual transaction in the Tx Pool.
The remedy is to directly rlp::encode the ethereum::Transaction structs returned by the runtime_api. Converting them to TransactionMessage is an unnecessary step.
Is there something left for follow-up PRs?
none
What alternative implementations were considered?
none
Are there relevant PRs or issues in other repositories (Substrate, Polkadot, Frontier, Cumulus)?
none
What value does it bring to the blockchain users?
Accurate pending/queued transaction Rpc.